Skip to content

fix: improve core stability and optional MCP websocket dependency handling#21

Open
wul48527-code wants to merge 3 commits intoHKUDS:mainfrom
wul48527-code:pr-a-stability-fixes
Open

fix: improve core stability and optional MCP websocket dependency handling#21
wul48527-code wants to merge 3 commits intoHKUDS:mainfrom
wul48527-code:pr-a-stability-fixes

Conversation

@wul48527-code
Copy link
Copy Markdown
Contributor

Background

This PR addresses several stability issues found during local usage:

  • MCP optional websocket dependency errors appearing during startup.
  • Core execution-path regressions around iteration propagation, registry path caching, workflow ID collisions, and UI summary rendering.
  • Missing regression tests for these behaviors.

Changes

  • MCP optional dependency handling:
    • Graceful degradation when websockets is missing.
    • Clear import-time guidance only when websocket transport is actually used.
  • Core stability fixes:
    • Ensure resolved max_iterations is passed in no-skill execution path.
    • Reuse dedicated skill-selection LLM client instead of recreating it repeatedly.
    • Normalize and validate auto-registered skill dirs before caching.
    • Avoid workflow summary ID collisions for duplicate leaf dir names across different roots.
    • Fix f-string syntax in UI summary output.

Tests

Added/updated regression tests:

  • tests/test_tool_layer.py
  • tests/test_mcp_server.py
  • tests/test_dashboard_server.py
  • tests/test_mcp_connectors_optional.py

Local validation:

  • py -3.13 -m unittest discover -s tests -v
  • Result: all tests passed.

Compatibility

The changes are backward compatible and primarily improve robustness and error handling.

@wul48527-code
Copy link
Copy Markdown
Contributor Author

补充测试说明(便于 review):

  • 本地环境:Windows + Python 3.13
  • 执行命令:
    py -3.13 -m unittest discover -s tests -v
  • 结果:全部通过(本 PR 分支 6/6)

本 PR 关注点:

  • MCP websocket 可选依赖缺失时的优雅降级
  • 执行路径与目录注册稳定性修复
  • workflow 汇总 ID 冲突修复
  • UI summary f-string 语法修复

如有需要,我可以把任意一个修复点再拆成更小 commit 方便逐个审阅。

@Dennis-yxchen
Copy link
Copy Markdown
Collaborator

Thanks for the thorough investigation. We've reviewed each item and cherry-picked the two changes that address real bugs into main:

  • max_iterations not forwarded in no-skill execution path — merged in 2fb8024 with regression tests
  • Workflow ID collision across roots — merged in the same commit, reworked to use a hash-based ID scheme instead of __-joined paths (the __ separator was ambiguous when directory names also contain __)

You're credited as co-author on both fixes.

Changes we're not including:

  • WebSocket graceful degradation — a missing optional dependency should fail loudly at the point of use, not silently swap in a stub class. Users need to know they need to pip install websockets, not discover it later.
  • LLMClient reuse in _get_skill_selection_llm — premature optimization; the client is lightweight and the current behavior is correct.
  • _normalize_skill_dir path caching — the existing behavior works; adding normalization introduces complexity without a demonstrated bug.
  • f-string syntax fix — Python 3.12+ (PEP 701) allows nested quotes in f-strings, so this is not a bug on supported versions.

Deepfreezechill referenced this pull request in Deepfreezechill/OpenSpace Apr 1, 2026
23 blocklist patterns, fail-closed, keyword arg detection, YAML bounds.

Closes #18, Closes #19, Closes #20, Closes #21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants